Skip to content

Conversation

@marcoAntonioNina
Copy link
Contributor

@marcoAntonioNina marcoAntonioNina commented Oct 23, 2025

Issue & Reproduction Steps

There not should be possible to upload dangerous files from WE

Solution

  • Introduced ValidatesFileTrait for file validation, ensuring security and type restrictions.

How to Test

Test

  • upload files in PM UI and API
  • upload files WebEntry
  • review API /files POST

Related Tickets & Packages

Code Review Checklist

  • I have pulled this code locally and tested it on my instance, along with any associated packages.
  • This code adheres to ProcessMaker Coding Guidelines.
  • This code includes a unit test or an E2E test that tests its functionality, or is covered by an existing test.
  • This solution fixes the bug reported in the original ticket.
  • This solution does not alter the expected output of a component in a way that would break existing Processes.
  • This solution does not implement any breaking changes that would invalidate documentation or cause existing Processes to fail.
  • This solution has been tested with enterprise packages that rely on its functionality and does not introduce bugs in those packages.
  • This code does not duplicate functionality that already exists in the framework or in ProcessMaker.
  • This ticket conforms to the PRD associated with this part of ProcessMaker.

ci:deploy
ci:package-webentry:bugfix/FOUR-26797
ci:package-collections:bugfix/FOUR-26797

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR is being reviewed by Cursor Bugbot

Details

Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.

To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.

// Validate MIME type vs extension if enabled
if (config('files.enable_mime_validation', true)) {
$this->validateExtensionMimeTypeMatch($file, $errors);
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: File Validation Defaults Cause Security Concerns

The file validation trait has inconsistent default config values. enable_dangerous_validation is disabled by default when not configured, but enable_extension_validation and enable_mime_validation default to true. This changes the previous opt-in behavior, potentially enabling validations unexpectedly and creating a security risk by silently skipping critical dangerous file checks.

Fix in Cursor Fix in Web

break;
}
}
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Inconsistent Error Handling in Validation Methods

The validatePDFFile method adds errors using an indexed array append ($errors[]), which differs from other validation methods in this trait that use an associative key ($errors['message']). This inconsistency can lead to unexpected error array structures, potentially breaking downstream error handling that expects a consistent 'message' key.

Fix in Cursor Fix in Web

private function validateFile(UploadedFile $file, &$errors)
{
// Explicitly reject archive files for security
if (config('files.enable_dangerous_validation')) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Default Validation Setting Insecure

The enable_dangerous_validation check in validateFile lacks a default true value, unlike other validation configurations. This results in dangerous file validation, like rejecting archive files, being disabled by default, which creates a security vulnerability.

Fix in Cursor Fix in Web

@vladyrichter
Copy link

QA server K8S was successfully deployed https://ci-fb8ea55a17.engk8s.processmaker.net

1 similar comment
@vladyrichter
Copy link

QA server K8S was successfully deployed https://ci-fb8ea55a17.engk8s.processmaker.net

@processmaker-sonarqube
Copy link

Quality Gate passed Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarQube

@pmPaulis
Copy link
Contributor

@marcoAntonioNina please check the comments from cursor, and add the corresponding comments if those are not applicable
Regards,

@nolanpro nolanpro changed the base branch from develop to 4.15.9+patch-a October 30, 2025 20:30
@nolanpro nolanpro merged commit 53ab967 into 4.15.9+patch-a Oct 30, 2025
23 of 34 checks passed
@nolanpro nolanpro deleted the bugfix/FOUR-26797 branch October 30, 2025 20:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants